test stream write cancellation support and beast2::write tests #102
Conversation
|
GCOVR code coverage report https://102.beast2.prtest3.cppalliance.org/gcovr/index.html Build time: 2025-12-02 14:25:14 UTC |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #102 +/- ##
===========================================
+ Coverage 53.19% 56.96% +3.77%
===========================================
Files 38 38
Lines 1690 1808 +118
===========================================
+ Hits 899 1030 +131
+ Misses 791 778 -13
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #102 +/- ##
===========================================
+ Coverage 53.19% 57.99% +4.80%
===========================================
Files 38 38
Lines 1690 1807 +117
===========================================
+ Hits 899 1048 +149
+ Misses 791 759 -32
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
please flatten this branch (no merge commits) |
Sure. Will do so before merging. |
include/boost/beast2/impl/write.hpp
Outdated
| BOOST_ASIO_HANDLER_LOCATION( | ||
| (__FILE__, __LINE__, "immediate")); | ||
| auto io_ex = self.get_io_executor(); | ||
| asio::async_immediate( |
There was a problem hiding this comment.
What is the reason for immediate completion?
There was a problem hiding this comment.
This code was taken from the pre-existing code at line 66 of this file (I'm not sure how to link that directly in github). Should it be calling the completion handler directly (with a goto upcall ) , and if so could that lead to infinite recursion ?
There was a problem hiding this comment.
The reason for using async_immediate in write_some is that it might complete without performing any I/O. However, in write operation we always call async_write_some at least once. Cancellation can't occur before the first call to async_write_some because everything until that point is synchronous (note that cancellation can't be emitted from another thread either).
| asio::cancellation_type::partial, | ||
| asio::cancellation_type::terminal }}; | ||
|
|
||
| for(auto ctype : ctypes) |
There was a problem hiding this comment.
What is the reason for testing all cancellation types? If operation supports total cancellation it automatically also support partial and terminal cancellation (it's a bitset).
There was a problem hiding this comment.
To ensure future behaviour remains consistent between them. It is always best to add tests for all possible scenarios even if we know the current code is not going to distinguish between them.
There was a problem hiding this comment.
Good. Generally speaking, if we test only with total cancellation, it implies that the other cases will work as well, since total cancellation provides the strongest guarantee. Changing the code so that it supports total cancellation but not, for example, partial cancellation is not trivial (creating custom cancellation filter), whereas the reverse is easy.
MungoG
force-pushed
the
issue-89-test-stream-cancellation-support
branch
from
4bb311b to
172e90a
Compare
4 participants
No description provided.